Skip to content

implement /api/schemas (json schemas from tsunami atoms /api/config /api/data)#2335

Merged
sawka merged 8 commits intomainfrom
sawka/tsunami-schema
Sep 12, 2025
Merged

implement /api/schemas (json schemas from tsunami atoms /api/config /api/data)#2335
sawka merged 8 commits intomainfrom
sawka/tsunami-schema

Conversation

@sawka
Copy link
Member

@sawka sawka commented Sep 12, 2025

No description provided.

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Sep 12, 2025

Caution

Review failed

The pull request is closed.

Walkthrough

Adds AtomMeta to app and engine packages and a Ptr helper; updates ConfigAtom/DataAtom signatures to accept optional AtomMeta. AtomImpl gains a meta field, GetMeta, and GetAtomType; MakeAtomImpl signature updated and callers adjusted. RootElem gains helper to list atoms by prefix and engine interfaces updated. Introduces a reflection-based JSON Schema generator (GenerateConfigSchema, GenerateDataSchema) that uses atom metadata and struct tags. HTTP server adds GET /api/schemas, applies no-cache headers to several endpoints, and registers the schemas handler. Multiple demo apps are updated to pass AtomMeta. util adds JSON tag parsing utilities and allows time.Time in JSON-serializability checks.

Estimated code review effort

🎯 5 (Critical) | ⏱️ ~120 minutes


📜 Recent review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 6aa0540 and 36c613e.

📒 Files selected for processing (2)
  • tsunami/engine/schema.go (1 hunks)
  • tsunami/prompts/system.md (7 hunks)
✨ Finishing touches
  • 📝 Generate Docstrings
🧪 Generate unit tests
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch sawka/tsunami-schema

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 7

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
tsunami/prompts/system.md (1)

695-697: Fix link element attribute. Should use href, not src.

This is user-facing and will confuse readers.

-vdom.H("link", map[string]any{"rel": "stylesheet", "src": "/static/mystyles.css"})
+vdom.H("link", map[string]any{"rel": "stylesheet", "href": "/static/mystyles.css"})
🧹 Nitpick comments (13)
tsunami/engine/rootelem.go (2)

86-99: Prefix filter helper is good; clarify key behavior in comment

Helper returns a snapshot with keys stripped of the prefix. Consider noting “keys are prefix-stripped” in the doc to avoid misuse.


110-117: Minor duplication: consider unifying GetDataMap/GetConfigMap

Both functions are identical except the prefix. A small helper like getAtomVals(prefix) could reduce duplication.

tsunami/engine/atomimpl.go (1)

12-20: Document AtomMeta immutability expectations

If callers shouldn’t mutate metadata after construction, add a brief comment stating it’s treated as immutable at runtime.

tsunami/util/util.go (1)

242-285: JSON tag parsing utility looks good; add tests and edge-case notes

Solid baseline. Recommend unit tests covering: json:"-", json:",omitempty,string", empty tag, renamed fields, unknown options retention, and embedded fields. If embedded/inline fields will be handled by schema code, document expectations here.

I can draft a small test table for ParseJSONTag; want me to add it?

tsunami/engine/serverhandlers.go (1)

265-291: Small hardening and client caching behavior.

  • Add defensive headers; optionally disable caching since schemas may change during runtime.

Apply within this block:

 w.Header().Set("Content-Type", "application/json")
+// schemas may change as atoms register/unregister; avoid stale caches
+w.Header().Set("Cache-Control", "no-store")
+// conservative hardening
+w.Header().Set("X-Content-Type-Options", "nosniff")
 if err := json.NewEncoder(w).Encode(result); err != nil {

If schema generation is non-trivial, consider caching the computed config/data schemas with an invalidation hook on atom registration to avoid recomputing on every GET.

tsunami/app/atom.go (1)

25-29: Atom[T] wrapper is clean.

API matches existing usage (Get/Set/SetFn). No issues.

To avoid false-positive mutation warnings when both current and new values are nil pointers, consider this tweak to sameRef (outside the changed lines):

func sameRef[T any](oldVal, newVal T) bool {
    vOld, vNew := reflect.ValueOf(oldVal), reflect.ValueOf(newVal)
    if !vOld.IsValid() || !vNew.IsValid() { return false }
    switch vNew.Kind() {
    case reflect.Ptr:
        if vOld.IsNil() || vNew.IsNil() { return false }
        return any(oldVal) == any(newVal)
    case reflect.Map, reflect.Slice:
        if vOld.Kind() != vNew.Kind() || vOld.IsZero() || vNew.IsZero() { return false }
        return vOld.Pointer() == vNew.Pointer()
    }
    return false
}
tsunami/demo/githubaction/app.go (1)

20-25: Well-chosen metadata across config/data atoms.

  • pollInterval: Units + min/max are appropriate.
  • repository/workflow: Patterns constrain inputs correctly.
  • maxWorkflowRuns: Reasonable bounds.
  • Data atoms: Helpful descriptions for observability.

UI copy says “Polls GitHub API every 5 seconds…” (Line 414) but poll interval is configurable; consider reflecting the current value or removing the hard-coded “5 seconds”.

Also applies to: 26-29, 30-33, 34-38, 39-41, 42-44, 45-47, 48-50

tsunami/demo/cpuchart/app.go (3)

14-18: Good use of AtomMeta with Min/Max; consider Units.

Optionally add Units: "%" for clarity in generated schemas.

-    Min:  app.Ptr(10.0),
-    Max:  app.Ptr(300.0),
+    Units: "%",
+    Min:   app.Ptr(10.0),
+    Max:   app.Ptr(300.0),

19-33: Add field-level schema tags to CPUDataPoint.

Improves generated schema (descriptions, units, ranges).

 type CPUDataPoint struct {
-    Time      int64    `json:"time"`      // Unix timestamp in seconds
-    CPUUsage  *float64 `json:"cpuUsage"`  // CPU usage percentage (nil for empty slots)
-    Timestamp string   `json:"timestamp"` // Human readable timestamp
+    Time      int64    `json:"time" desc:"Unix timestamp (seconds since epoch)" units:"s"`
+    CPUUsage  *float64 `json:"cpuUsage" desc:"CPU usage percentage" units:"%" min:"0" max:"100"`
+    Timestamp string   `json:"timestamp" desc:"Human-readable HH:MM:SS"`
 }

Also applies to: 36-40


71-73: Reduce noisy logs in tight ticker loop.

Per-second logs will spam debug output. Gate behind a verbose flag or reduce frequency.

- log.Printf("CPU Usage: %.2f%% at %s", cpuUsage, dataPoint.Timestamp)
+ if false { // TODO: wire to a verbose flag
+     log.Printf("CPU Usage: %.2f%% at %s", cpuUsage, dataPoint.Timestamp)
+ }

Also applies to: 153-161

tsunami/prompts/system.md (2)

459-470: Clarify AtomMeta parameter is optional (nil allowed).

Text says “require”; API accepts nil. Adjust wording to prevent confusion.

-ConfigAtom and DataAtom require an AtomMeta parameter
+ConfigAtom and DataAtom accept an AtomMeta parameter (pass nil if not needed)

1233-1240: Replace hard tabs with spaces to satisfy markdownlint (MD010).

Multiple lines flagged by markdownlint-cli2.

Also applies to: 1270-1290, 1292-1299, 1305-1318, 1336-1336

tsunami/engine/schema.go (1)

273-281: Consider adding “required” at root only if needed.

Currently all atoms are optional; that’s fine. If you later need required config keys, introduce a Required slice in metadata.

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 2783eeb and 4259018.

📒 Files selected for processing (15)
  • tsunami/app/atom.go (1 hunks)
  • tsunami/app/defaultclient.go (2 hunks)
  • tsunami/demo/cpuchart/app.go (2 hunks)
  • tsunami/demo/githubaction/app.go (1 hunks)
  • tsunami/demo/pomodoro/app.go (3 hunks)
  • tsunami/demo/recharts/app.go (1 hunks)
  • tsunami/demo/tabletest/app.go (1 hunks)
  • tsunami/demo/tsunamiconfig/app.go (1 hunks)
  • tsunami/engine/atomimpl.go (2 hunks)
  • tsunami/engine/hooks.go (1 hunks)
  • tsunami/engine/rootelem.go (2 hunks)
  • tsunami/engine/schema.go (1 hunks)
  • tsunami/engine/serverhandlers.go (2 hunks)
  • tsunami/prompts/system.md (6 hunks)
  • tsunami/util/util.go (4 hunks)
🧰 Additional context used
🧬 Code graph analysis (13)
tsunami/engine/schema.go (3)
tsunami/util/util.go (1)
  • ParseJSONTag (249-285)
tsunami/engine/atomimpl.go (1)
  • AtomMeta (13-20)
tsunami/engine/rootelem.go (1)
  • RootElem (35-48)
tsunami/engine/serverhandlers.go (2)
tsunami/util/util.go (1)
  • PanicHandler (25-35)
tsunami/engine/schema.go (2)
  • GenerateConfigSchema (295-298)
  • GenerateDataSchema (301-304)
tsunami/engine/hooks.go (1)
tsunami/engine/atomimpl.go (1)
  • MakeAtomImpl (29-36)
tsunami/engine/atomimpl.go (1)
tsunami/app/atom.go (1)
  • AtomMeta (16-23)
tsunami/engine/rootelem.go (1)
tsunami/engine/atomimpl.go (1)
  • AtomMeta (13-20)
tsunami/demo/recharts/app.go (3)
tsunami/app/defaultclient.go (2)
  • DataAtom (51-58)
  • ConfigAtom (42-49)
tsunami/engine/atomimpl.go (1)
  • AtomMeta (13-20)
tsunami/app/atom.go (1)
  • AtomMeta (16-23)
tsunami/app/atom.go (1)
tsunami/engine/clientimpl.go (1)
  • ClientImpl (37-61)
tsunami/demo/tabletest/app.go (2)
tsunami/engine/atomimpl.go (1)
  • AtomMeta (13-20)
tsunami/app/atom.go (1)
  • AtomMeta (16-23)
tsunami/demo/githubaction/app.go (3)
tsunami/app/defaultclient.go (3)
  • ConfigAtom (42-49)
  • Ptr (19-21)
  • DataAtom (51-58)
tsunami/engine/atomimpl.go (1)
  • AtomMeta (13-20)
tsunami/app/atom.go (1)
  • AtomMeta (16-23)
tsunami/demo/cpuchart/app.go (3)
tsunami/app/defaultclient.go (3)
  • ConfigAtom (42-49)
  • Ptr (19-21)
  • DataAtom (51-58)
tsunami/engine/atomimpl.go (1)
  • AtomMeta (13-20)
tsunami/app/atom.go (1)
  • AtomMeta (16-23)
tsunami/app/defaultclient.go (2)
tsunami/engine/atomimpl.go (2)
  • AtomMeta (13-20)
  • MakeAtomImpl (29-36)
tsunami/app/atom.go (2)
  • AtomMeta (16-23)
  • Atom (26-29)
tsunami/demo/pomodoro/app.go (3)
tsunami/app/defaultclient.go (2)
  • DataAtom (51-58)
  • Ptr (19-21)
tsunami/engine/atomimpl.go (1)
  • AtomMeta (13-20)
tsunami/app/atom.go (1)
  • AtomMeta (16-23)
tsunami/demo/tsunamiconfig/app.go (3)
tsunami/app/defaultclient.go (1)
  • ConfigAtom (42-49)
tsunami/engine/atomimpl.go (1)
  • AtomMeta (13-20)
tsunami/app/atom.go (1)
  • AtomMeta (16-23)
🪛 markdownlint-cli2 (0.17.2)
tsunami/prompts/system.md

1235-1235: Hard tabs
Column: 1

(MD010, no-hard-tabs)


1236-1236: Hard tabs
Column: 1

(MD010, no-hard-tabs)


1238-1238: Hard tabs
Column: 1

(MD010, no-hard-tabs)


1270-1270: Hard tabs
Column: 1

(MD010, no-hard-tabs)


1271-1271: Hard tabs
Column: 1

(MD010, no-hard-tabs)


1272-1272: Hard tabs
Column: 1

(MD010, no-hard-tabs)


1274-1274: Hard tabs
Column: 1

(MD010, no-hard-tabs)


1275-1275: Hard tabs
Column: 1

(MD010, no-hard-tabs)


1276-1276: Hard tabs
Column: 1

(MD010, no-hard-tabs)


1277-1277: Hard tabs
Column: 1

(MD010, no-hard-tabs)


1278-1278: Hard tabs
Column: 1

(MD010, no-hard-tabs)


1279-1279: Hard tabs
Column: 1

(MD010, no-hard-tabs)


1280-1280: Hard tabs
Column: 1

(MD010, no-hard-tabs)


1281-1281: Hard tabs
Column: 1

(MD010, no-hard-tabs)


1283-1283: Hard tabs
Column: 1

(MD010, no-hard-tabs)


1284-1284: Hard tabs
Column: 1

(MD010, no-hard-tabs)


1285-1285: Hard tabs
Column: 1

(MD010, no-hard-tabs)


1286-1286: Hard tabs
Column: 1

(MD010, no-hard-tabs)


1287-1287: Hard tabs
Column: 1

(MD010, no-hard-tabs)


1288-1288: Hard tabs
Column: 1

(MD010, no-hard-tabs)


1289-1289: Hard tabs
Column: 1

(MD010, no-hard-tabs)


1290-1290: Hard tabs
Column: 1

(MD010, no-hard-tabs)


1292-1292: Hard tabs
Column: 1

(MD010, no-hard-tabs)


1293-1293: Hard tabs
Column: 1

(MD010, no-hard-tabs)


1294-1294: Hard tabs
Column: 1

(MD010, no-hard-tabs)


1295-1295: Hard tabs
Column: 1

(MD010, no-hard-tabs)


1296-1296: Hard tabs
Column: 1

(MD010, no-hard-tabs)


1297-1297: Hard tabs
Column: 1

(MD010, no-hard-tabs)


1298-1298: Hard tabs
Column: 1

(MD010, no-hard-tabs)


1299-1299: Hard tabs
Column: 1

(MD010, no-hard-tabs)


1300-1300: Hard tabs
Column: 1

(MD010, no-hard-tabs)


1301-1301: Hard tabs
Column: 1

(MD010, no-hard-tabs)


1302-1302: Hard tabs
Column: 1

(MD010, no-hard-tabs)


1303-1303: Hard tabs
Column: 1

(MD010, no-hard-tabs)


1305-1305: Hard tabs
Column: 1

(MD010, no-hard-tabs)


1306-1306: Hard tabs
Column: 1

(MD010, no-hard-tabs)


1307-1307: Hard tabs
Column: 1

(MD010, no-hard-tabs)


1308-1308: Hard tabs
Column: 1

(MD010, no-hard-tabs)


1309-1309: Hard tabs
Column: 1

(MD010, no-hard-tabs)


1310-1310: Hard tabs
Column: 1

(MD010, no-hard-tabs)


1311-1311: Hard tabs
Column: 1

(MD010, no-hard-tabs)


1312-1312: Hard tabs
Column: 1

(MD010, no-hard-tabs)


1313-1313: Hard tabs
Column: 1

(MD010, no-hard-tabs)


1314-1314: Hard tabs
Column: 1

(MD010, no-hard-tabs)


1316-1316: Hard tabs
Column: 1

(MD010, no-hard-tabs)


1317-1317: Hard tabs
Column: 1

(MD010, no-hard-tabs)


1318-1318: Hard tabs
Column: 1

(MD010, no-hard-tabs)


1336-1336: Hard tabs
Column: 1

(MD010, no-hard-tabs)

🔇 Additional comments (15)
tsunami/engine/rootelem.go (2)

101-108: Avoid holding atomLock while calling GetVal — good

Collecting under lock then calling GetVal() outside reduces contention and deadlock risk. LGTM.


27-33: Interface expansion: GetMeta added — verified; no action required

AtomImpl[T] implements GetMeta/GetVal/SetVal/SetUsedBy/GetUsedBy (tsunami/engine/atomimpl.go) and all RegisterAtom call sites use MakeAtomImpl(...) (tsunami/app/defaultclient.go, tsunami/engine/hooks.go); no custom/mocked atom implementations found — no compile break expected.

tsunami/util/util.go (2)

127-128: Allowing time.Time through JSON-serializable check is appropriate

Explicit timeType plus early allow in ValidateAtomType is correct and handles pointers/slices via recursion.

Also applies to: 168-171


21-24: Comment reflow for PanicHandler example — fine

No functional changes.

tsunami/engine/hooks.go (1)

75-83: Signature update applied correctly

MakeAtomImpl usages match the new constructor; call sites found:

  • tsunami/engine/hooks.go:80 — MakeAtomImpl(initialVal, nil)
  • tsunami/app/defaultclient.go:46 — engine.MakeAtomImpl(defaultValue, engineMeta)
  • tsunami/app/defaultclient.go:55 — engine.MakeAtomImpl(defaultValue, engineMeta)
  • tsunami/app/defaultclient.go:63 — engine.MakeAtomImpl(defaultValue, nil)
tsunami/demo/tabletest/app.go (1)

34-36: Metadata wiring for sampleData looks good

Good example metadata; this will surface in schemas.

tsunami/engine/serverhandlers.go (1)

51-51: Registering /api/schemas looks correct.

Endpoint wiring matches existing pattern. No concerns.

tsunami/app/atom.go (1)

15-23: AtomMeta shape looks good and minimal.

Fields align with engine meta after conversion; good public surface.

tsunami/demo/pomodoro/app.go (2)

21-26: Nice use of metadata on remainingSeconds.

Desc/Units/Min/Max are sensible for schema/validation.


197-197: Minor prop alignment change is fine.

No behavioral impact.

tsunami/demo/tsunamiconfig/app.go (1)

19-22: Good ConfigAtom metadata (Desc + Pattern).

Pattern matches the parsing behavior (port-only, host:port, or full URL).

tsunami/demo/recharts/app.go (1)

13-22: Metadata adds clarity and constraints.

  • chartDataAtom: Doc-only meta looks good.
  • chartTypeAtom: Enum aligns with UI select options.
  • isAnimatingAtom: Clear description.
tsunami/app/defaultclient.go (2)

19-21: Ptr helper is correct and useful.

Escapes correctly; works well for AtomMeta Min/Max.


63-66: Metadata plumbing LGTM.

Nil-safe conversion and full field mapping (Desc→Description, Units/Min/Max/Enum/Pattern) look correct.

Also applies to: 68-80

tsunami/engine/schema.go (1)

300-304: Endpoint schemas LGTM.

Clear separation of Config vs Data; titles/descriptions set.

Comment on lines +42 to 49
func ConfigAtom[T any](name string, defaultValue T, meta *AtomMeta) Atom[T] {
fullName := "$config." + name
client := engine.GetDefaultClient()
atom := engine.MakeAtomImpl(defaultValue)
engineMeta := convertAppMetaToEngineMeta(meta)
atom := engine.MakeAtomImpl(defaultValue, engineMeta)
client.Root.RegisterAtom(fullName, atom)
return Atom[T]{name: fullName, client: client}
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Verification agent

🧩 Analysis chain

Backwards-incompatible API change: ensure all call sites add the meta arg.

ConfigAtom/DataAtom now require a third parameter. Verify no two-arg calls remain.

Run:

Also applies to: 51-58


🏁 Script executed:

#!/bin/bash
# Find legacy two-arg calls to ConfigAtom/DataAtom
rg -nP --type=go -C1 '\bConfigAtom\s*\(\s*[^,]+,\s*[^,]+\s*\)' | rg -v '&app\.AtomMeta|\bnil\s*\)'
rg -nP --type=go -C1 '\bDataAtom\s*\(\s*[^,]+,\s*[^,]+\s*\)'   | rg -v '&app\.AtomMeta|\bnil\s*\)'

Length of output: 370


Backwards-incompatible API change — add missing meta argument to ConfigAtom/DataAtom call sites

Found a legacy two-arg call; update to pass the new third meta parameter.

  • tsunami/demo/cpuchart/app.go:19 — cpuDataAtom = app.DataAtom("cpuData", func() []CPUDataPoint { ... })
🤖 Prompt for AI Agents
In tsunami/app/defaultclient.go around lines 42-49 the ConfigAtom signature now
requires a third meta parameter; update legacy two-arg call sites (e.g.,
tsunami/demo/cpuchart/app.go:19) to pass the new meta argument—either nil if no
metadata is needed or a constructed &AtomMeta{...} with the required fields. Do
the same for DataAtom call sites: add the meta parameter to each call, recompile
and run tests to ensure no remaining references to the old two-argument form.

Comment on lines +29 to 36
func MakeAtomImpl[T any](initialVal T, meta *AtomMeta) *AtomImpl[T] {
return &AtomImpl[T]{
lock: &sync.Mutex{},
val: initialVal,
usedBy: make(map[string]bool),
meta: meta,
}
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

Meta escape hatch: copy metadata at construction to prevent external mutation

Storing the caller-provided *AtomMeta directly allows later unsynchronized mutations by the caller, leading to data races and inconsistent schemas. Clone it on input.

Apply:

 func MakeAtomImpl[T any](initialVal T, meta *AtomMeta) *AtomImpl[T] {
 	return &AtomImpl[T]{
 		lock:   &sync.Mutex{},
 		val:    initialVal,
 		usedBy: make(map[string]bool),
-		meta:   meta,
+		meta:   cloneAtomMeta(meta),
 	}
 }

Add once in this file:

// cloneAtomMeta makes a deep copy to keep AtomMeta immutable/owned by the atom.
func cloneAtomMeta(in *AtomMeta) *AtomMeta {
	if in == nil {
		return nil
	}
	out := *in // shallow copy first
	if in.Enum != nil {
		out.Enum = append([]string(nil), in.Enum...)
	}
	if in.Min != nil {
		v := *in.Min
		out.Min = &v
	}
	if in.Max != nil {
		v := *in.Max
		out.Max = &v
	}
	return &out
}
🤖 Prompt for AI Agents
In tsunami/engine/atomimpl.go around lines 29 to 36, the AtomImpl constructor
stores the caller-provided *AtomMeta pointer directly which allows external
mutation and data races; add a cloneAtomMeta helper in this file that
deep-copies AtomMeta (copy struct, clone Enum slice, copy Min/Max pointers) and
call cloneAtomMeta(meta) when constructing AtomImpl so the atom owns an
immutable copy of the metadata.

Comment on lines +100 to +104
func (a *AtomImpl[T]) GetMeta() *AtomMeta {
a.lock.Lock()
defer a.lock.Unlock()
return a.meta
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

Do not return internal pointer; return a clone to avoid data races

Returning a.meta exposes internal state for mutation without locks. Return a cloned copy.

Apply:

 func (a *AtomImpl[T]) GetMeta() *AtomMeta {
 	a.lock.Lock()
 	defer a.lock.Unlock()
-	return a.meta
+	return cloneAtomMeta(a.meta)
 }

Committable suggestion skipped: line range outside the PR's diff.

🤖 Prompt for AI Agents
In tsunami/engine/atomimpl.go around lines 100-104, the method currently returns
the internal pointer a.meta which exposes internal state; modify GetMeta to hold
the lock, defensively handle nil, create a cloned copy of the AtomMeta while
locked (e.g., metaCopy := *a.meta; return &metaCopy) or call/implement an
AtomMeta.Clone() that deep-copies any reference fields, then return that clone
so callers cannot mutate internal state outside the lock.

Comment on lines +224 to +226
schema["$ref"] = fmt.Sprintf("#/definitions/%s", t.Name())
case reflect.Ptr:
return generateShallowJSONSchema(t.Elem(), meta)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

Avoid definition name collisions; use fully-qualified names.

t.Name() can collide across packages; use PkgPath()+Name (sanitized) consistently for $ref and definitions.

-        schema["$ref"] = fmt.Sprintf("#/definitions/%s", t.Name())
+        schema["$ref"] = fmt.Sprintf("#/definitions/%s", defKey(t))
-    for t, def := range defs {
-        definitions[t.Name()] = def
-    }
+    for t, def := range defs {
+        definitions[defKey(t)] = def
+    }

Add helper (outside current range):

// defKey builds a stable, unique key for type definitions.
func defKey(t reflect.Type) string {
    if t == nil {
        return ""
    }
    if t.PkgPath() == "" {
        return t.Name()
    }
    key := t.PkgPath() + "." + t.Name()
    key = strings.ReplaceAll(key, "/", ".")
    return key
}

Also applies to: 285-288

🤖 Prompt for AI Agents
In tsunami/engine/schema.go around lines 224-226 (and also apply at ~285-288),
the code uses t.Name() for JSON Schema $ref/definitions which can collide across
packages; add a helper defKey(reflect.Type) that returns PkgPath()+". "+Name
with slashes replaced by dots (handle empty PkgPath by returning Name) and
replace uses of t.Name() with defKey(t) when building schema["$ref"] and when
creating/looking up entries in definitions; ensure the helper is declared
outside the current function range and used consistently for pointer unwrapping
and definition lookups.

Comment on lines +239 to +245
func getAtomValueType(atom genAtom) reflect.Type {
val := atom.GetVal()
if val == nil {
return nil
}
return reflect.TypeOf(val)
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

Atoms with nil default values vanish from schema.

getAtomValueType returns nil when current value is nil (e.g., nil slice/pointer), dropping properties. Use a stored generic type as fallback.

 func getAtomValueType(atom genAtom) reflect.Type {
     val := atom.GetVal()
-    if val == nil {
-        return nil
-    }
-    return reflect.TypeOf(val)
+    if val != nil {
+        return reflect.TypeOf(val)
+    }
+    // Fallback to the declared atom type (requires genAtom to expose it)
+    if t := atom.GetValType(); t != nil {
+        return t
+    }
+    return nil
 }

Follow-up change outside this file (proposed):

// In engine/atomimpl.go
type AtomImpl[T any] struct {
    // ...
    valType reflect.Type
}

func MakeAtomImpl[T any](initialVal T, meta *AtomMeta) *AtomImpl[T] {
    return &AtomImpl[T]{
        // ...
        val:     initialVal,
        meta:    meta,
        valType: reflect.TypeOf((*T)(nil)).Elem(), // robust even if initialVal is nil
    }
}

// In genAtom interface and AtomImpl methods:
type genAtom interface {
    GetVal() any
    GetMeta() *AtomMeta
    GetValType() reflect.Type
}

func (a *AtomImpl[T]) GetValType() reflect.Type { return a.valType }

Also applies to: 256-271

🤖 Prompt for AI Agents
In tsunami/engine/schema.go around lines 239-245 (also apply same change for
256-271), getAtomValueType currently returns nil when atom.GetVal() is nil which
drops properties; instead call a new genAtom method GetValType() to obtain a
stored generic reflect.Type fallback when the runtime value is nil. Update
getAtomValueType to first check val := atom.GetVal(); if val != nil return
reflect.TypeOf(val) else return atom.GetValType(); implement the follow-up
changes in engine/atomimpl.go to store valType in AtomImpl[T] (initialized via
reflect.TypeOf((*T)(nil)).Elem()) and expose it via GetValType() on the genAtom
interface and AtomImpl.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 4

♻️ Duplicate comments (2)
tsunami/engine/atomimpl.go (2)

101-106: Do not return internal pointer; return a clone

Expose a copy to avoid external mutation of internal state.

 func (a *AtomImpl[T]) GetMeta() *AtomMeta {
     a.lock.Lock()
     defer a.lock.Unlock()
-    return a.meta
+    return cloneAtomMeta(a.meta)
 }

30-36: Own the metadata: clone on construction to prevent external mutation/data races

Storing caller-provided meta directly allows unsynchronized mutation and schema drift. Clone it at construction.

 func MakeAtomImpl[T any](initialVal T, meta *AtomMeta) *AtomImpl[T] {
-    return &AtomImpl[T]{
-        lock:   &sync.Mutex{},
-        val:    initialVal,
-        usedBy: make(map[string]bool),
-        meta:   meta,
-    }
+    return &AtomImpl[T]{
+        lock:    &sync.Mutex{},
+        val:     initialVal,
+        usedBy:  make(map[string]bool),
+        meta:    cloneAtomMeta(meta),
+        valType: reflect.TypeOf((*T)(nil)).Elem(),
+    }
 }
+
+// cloneAtomMeta makes a deep copy so AtomImpl owns an immutable snapshot.
+func cloneAtomMeta(in *AtomMeta) *AtomMeta {
+    if in == nil {
+        return nil
+    }
+    out := *in
+    if in.Enum != nil {
+        out.Enum = append([]string(nil), in.Enum...)
+    }
+    if in.Min != nil {
+        v := *in.Min
+        out.Min = &v
+    }
+    if in.Max != nil {
+        v := *in.Max
+        out.Max = &v
+    }
+    return &out
+}
🧹 Nitpick comments (10)
tsunami/engine/rootelem.go (1)

87-101: Minor: pre-size the result map

Micro-optimization: you can pre-size result to reduce rehashing.

- result := make(map[string]genAtom)
+ result := make(map[string]genAtom, len(r.Atoms))
tsunami/engine/serverhandlers.go (1)

277-305: /api/schemas handler: add Allow header on 405

For stricter HTTP semantics, advertise allowed methods when returning 405.

- if r.Method != http.MethodGet {
-     http.Error(w, "method not allowed", http.StatusMethodNotAllowed)
+ if r.Method != http.MethodGet {
+     w.Header().Set("Allow", http.MethodGet)
+     http.Error(w, "method not allowed", http.StatusMethodNotAllowed)
      return
  }
tsunami/demo/githubaction/app.go (1)

320-327: Use local isLoading variable instead of repeated atom.Get() calls

Reduces repeated reads and improves readability.

-                                "className": vdom.Classes(
-                                    "px-4 py-2 rounded-md text-sm font-medium transition-colors cursor-pointer",
-                                    vdom.IfElse(isLoadingAtom.Get(), "bg-gray-600 text-gray-400", "bg-blue-600 hover:bg-blue-700 text-white"),
-                                ),
-                                "onClick":  vdom.If(!isLoadingAtom.Get(), handleRefresh),
-                                "disabled": isLoadingAtom.Get(),
+                                "className": vdom.Classes(
+                                    "px-4 py-2 rounded-md text-sm font-medium transition-colors cursor-pointer",
+                                    vdom.IfElse(isLoading, "bg-gray-600 text-gray-400", "bg-blue-600 hover:bg-blue-700 text-white"),
+                                ),
+                                "onClick":  vdom.If(!isLoading, handleRefresh),
+                                "disabled": isLoading,
tsunami/prompts/system.md (5)

482-495: Use the proper package qualifier for AtomMeta in examples.

Examples currently use &AtomMeta{...} which can confuse readers. Prefer &app.AtomMeta{...} to match the actual API.

-    theme = app.ConfigAtom("theme", "dark", &AtomMeta{
+    theme = app.ConfigAtom("theme", "dark", &app.AtomMeta{
@@
-    apiKey = app.ConfigAtom("apiKey", "", &AtomMeta{
+    apiKey = app.ConfigAtom("apiKey", "", &app.AtomMeta{
@@
-    maxRetries = app.ConfigAtom("maxRetries", 3, &AtomMeta{
+    maxRetries = app.ConfigAtom("maxRetries", 3, &app.AtomMeta{

497-503: Same package-qualification fix for DataAtom examples.

-    currentUser = app.DataAtom("currentUser", UserStats{}, &AtomMeta{
+    currentUser = app.DataAtom("currentUser", UserStats{}, &app.AtomMeta{
@@
-    lastPollResult = app.DataAtom("lastPoll", APIResult{}, &AtomMeta{
+    lastPollResult = app.DataAtom("lastPoll", APIResult{}, &app.AtomMeta{

541-542: Document minimal response shape for /api/schemas.

Add a tiny example to reduce guesswork for integrators (top-level keys and where config vs data land). I can draft one if helpful.


550-559: Keep AtomMeta usage consistent here, too.

-userPrefs := app.ConfigAtom("userPrefs", UserPrefs{}, &AtomMeta{
+userPrefs := app.ConfigAtom("userPrefs", UserPrefs{}, &app.AtomMeta{

1235-1336: Resolve markdownlint MD010 (hard tabs).

The linter flags hard tabs across code examples. Either convert leading tabs to spaces in fenced blocks or disable MD010 for code fences in your markdownlint config:

  • Convert: replace leading tabs in code blocks with 4 spaces.
  • Or config: set MD010.code_blocks=false for this file.

Run your markdownlint locally to confirm the warnings are cleared.

tsunami/demo/cpuchart/app.go (2)

19-33: Avoid hard-coded 60 in initial data; derive from config to prevent first-tick resize.

Using a literal 60 forces a resize on the first tick when config differs. If safe at init time, read the current config:

-        dataPointCount := 60 // Default value for initialization
+        dataPointCount := dataPointCountAtom.Get() // Use configured window size

If calling Get() at package init is unsafe in your lifecycle, initialize with an empty slice here and seed size in App via a one-time effect that sets the slice to dataPointCountAtom.Get().


71-73: Reduce per-second logging noise.

Log each Nth sample or behind a debug flag to keep logs usable.

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 4259018 and 4fba48f.

📒 Files selected for processing (7)
  • tsunami/demo/cpuchart/app.go (4 hunks)
  • tsunami/demo/githubaction/app.go (2 hunks)
  • tsunami/engine/atomimpl.go (2 hunks)
  • tsunami/engine/rootelem.go (2 hunks)
  • tsunami/engine/schema.go (1 hunks)
  • tsunami/engine/serverhandlers.go (7 hunks)
  • tsunami/prompts/system.md (7 hunks)
🧰 Additional context used
🧬 Code graph analysis (6)
tsunami/engine/schema.go (3)
tsunami/util/util.go (1)
  • ParseJSONTag (249-285)
tsunami/engine/atomimpl.go (1)
  • AtomMeta (14-21)
tsunami/engine/rootelem.go (1)
  • RootElem (36-49)
tsunami/engine/rootelem.go (2)
tsunami/engine/atomimpl.go (1)
  • AtomMeta (14-21)
tsunami/app/atom.go (1)
  • AtomMeta (16-23)
tsunami/engine/atomimpl.go (2)
tsunami/app/atom.go (1)
  • AtomMeta (16-23)
pkg/waveobj/waveobj.go (1)
  • GetMeta (216-226)
tsunami/demo/githubaction/app.go (3)
tsunami/app/defaultclient.go (3)
  • ConfigAtom (42-49)
  • Ptr (19-21)
  • DataAtom (51-58)
tsunami/engine/atomimpl.go (1)
  • AtomMeta (14-21)
tsunami/app/atom.go (1)
  • AtomMeta (16-23)
tsunami/engine/serverhandlers.go (2)
tsunami/util/util.go (1)
  • PanicHandler (25-35)
tsunami/engine/schema.go (2)
  • GenerateConfigSchema (292-295)
  • GenerateDataSchema (298-301)
tsunami/demo/cpuchart/app.go (3)
tsunami/app/defaultclient.go (3)
  • ConfigAtom (42-49)
  • Ptr (19-21)
  • DataAtom (51-58)
tsunami/engine/atomimpl.go (1)
  • AtomMeta (14-21)
tsunami/app/atom.go (1)
  • AtomMeta (16-23)
🪛 markdownlint-cli2 (0.17.2)
tsunami/prompts/system.md

1235-1235: Hard tabs
Column: 1

(MD010, no-hard-tabs)


1236-1236: Hard tabs
Column: 1

(MD010, no-hard-tabs)


1238-1238: Hard tabs
Column: 1

(MD010, no-hard-tabs)


1270-1270: Hard tabs
Column: 1

(MD010, no-hard-tabs)


1271-1271: Hard tabs
Column: 1

(MD010, no-hard-tabs)


1272-1272: Hard tabs
Column: 1

(MD010, no-hard-tabs)


1274-1274: Hard tabs
Column: 1

(MD010, no-hard-tabs)


1275-1275: Hard tabs
Column: 1

(MD010, no-hard-tabs)


1276-1276: Hard tabs
Column: 1

(MD010, no-hard-tabs)


1277-1277: Hard tabs
Column: 1

(MD010, no-hard-tabs)


1278-1278: Hard tabs
Column: 1

(MD010, no-hard-tabs)


1279-1279: Hard tabs
Column: 1

(MD010, no-hard-tabs)


1280-1280: Hard tabs
Column: 1

(MD010, no-hard-tabs)


1281-1281: Hard tabs
Column: 1

(MD010, no-hard-tabs)


1283-1283: Hard tabs
Column: 1

(MD010, no-hard-tabs)


1284-1284: Hard tabs
Column: 1

(MD010, no-hard-tabs)


1285-1285: Hard tabs
Column: 1

(MD010, no-hard-tabs)


1286-1286: Hard tabs
Column: 1

(MD010, no-hard-tabs)


1287-1287: Hard tabs
Column: 1

(MD010, no-hard-tabs)


1288-1288: Hard tabs
Column: 1

(MD010, no-hard-tabs)


1289-1289: Hard tabs
Column: 1

(MD010, no-hard-tabs)


1290-1290: Hard tabs
Column: 1

(MD010, no-hard-tabs)


1292-1292: Hard tabs
Column: 1

(MD010, no-hard-tabs)


1293-1293: Hard tabs
Column: 1

(MD010, no-hard-tabs)


1294-1294: Hard tabs
Column: 1

(MD010, no-hard-tabs)


1295-1295: Hard tabs
Column: 1

(MD010, no-hard-tabs)


1296-1296: Hard tabs
Column: 1

(MD010, no-hard-tabs)


1297-1297: Hard tabs
Column: 1

(MD010, no-hard-tabs)


1298-1298: Hard tabs
Column: 1

(MD010, no-hard-tabs)


1299-1299: Hard tabs
Column: 1

(MD010, no-hard-tabs)


1300-1300: Hard tabs
Column: 1

(MD010, no-hard-tabs)


1301-1301: Hard tabs
Column: 1

(MD010, no-hard-tabs)


1302-1302: Hard tabs
Column: 1

(MD010, no-hard-tabs)


1303-1303: Hard tabs
Column: 1

(MD010, no-hard-tabs)


1305-1305: Hard tabs
Column: 1

(MD010, no-hard-tabs)


1306-1306: Hard tabs
Column: 1

(MD010, no-hard-tabs)


1307-1307: Hard tabs
Column: 1

(MD010, no-hard-tabs)


1308-1308: Hard tabs
Column: 1

(MD010, no-hard-tabs)


1309-1309: Hard tabs
Column: 1

(MD010, no-hard-tabs)


1310-1310: Hard tabs
Column: 1

(MD010, no-hard-tabs)


1311-1311: Hard tabs
Column: 1

(MD010, no-hard-tabs)


1312-1312: Hard tabs
Column: 1

(MD010, no-hard-tabs)


1313-1313: Hard tabs
Column: 1

(MD010, no-hard-tabs)


1314-1314: Hard tabs
Column: 1

(MD010, no-hard-tabs)


1316-1316: Hard tabs
Column: 1

(MD010, no-hard-tabs)


1317-1317: Hard tabs
Column: 1

(MD010, no-hard-tabs)


1318-1318: Hard tabs
Column: 1

(MD010, no-hard-tabs)


1336-1336: Hard tabs
Column: 1

(MD010, no-hard-tabs)

🔇 Additional comments (17)
tsunami/engine/rootelem.go (3)

102-109: GetDataMap: clearer, safer iteration via helper — LGTM


111-118: GetConfigMap: consistent with GetDataMap — LGTM


32-34: Interface extension verified — AtomImpl implements GetMeta & GetAtomType

tsunami/engine/atomimpl.go defines (a *AtomImpl[T]) GetMeta() *AtomMeta and (a *AtomImpl[T]) GetAtomType() reflect.Type; no other genAtom implementers were found.

tsunami/engine/serverhandlers.go (3)

46-51: Centralized no-cache helper — LGTM


57-57: Register /api/schemas — LGTM


80-81: Applying no-cache across JSON/SSE endpoints — LGTM

Also applies to: 192-193, 216-217, 344-345, 457-458

tsunami/demo/githubaction/app.go (2)

20-50: Metadata wiring for atoms — LGTM

Good use of Desc/Units/Pattern/Min/Max; this will surface cleanly in /api/schemas.


414-415: Nice touch on dynamic copy

Switching to “Polls GitHub API every {pollInterval} seconds” improves UX clarity.

tsunami/engine/schema.go (2)

81-86: Field-level pattern tag support — LGTM


202-207: Correct use of contentEncoding/contentMediaType for []byte — LGTM

tsunami/prompts/system.md (2)

693-696: LGTM: correct attribute on link tag.

srshref is the right fix.


1233-1240: LGTM: good DataAtom example with app.AtomMeta.

Clear demo of meta usage and external integration.

tsunami/demo/cpuchart/app.go (5)

14-18: LGTM: ConfigAtom metadata and constraints look good.

Nice use of Min/Max via app.Ptr and concise Desc.


37-39: LGTM: schema tags on CPUDataPoint fields are precise.

Desc/units/min/max will generate a clear JSON Schema, and pointer type for CPUUsage correctly models nulls.


46-55: Confirm timer semantics with cpu.Percent + UseTicker.

cpu.Percent(time.Second, false) blocks ~1s. Ensure UseTicker’s execution model won’t backlog ticks if the callback occasionally exceeds 1s (time.Ticker channel drop semantics typically avoid backlog). If needed, consider interval 0 with prior-sample semantics.


160-179: LGTM: resize logic preserves recent data correctly.

Right-align copy into the resized buffer and trim with a tail slice—clear and correct.


216-217: LGTM: dynamic text wired to current window size.

Nice touch keeping UI copy in sync with config.

Also applies to: 343-344

Comment on lines +231 to +233
// Reference the definition instead of recursing
schema["$ref"] = fmt.Sprintf("#/definitions/%s", t.Name())
case reflect.Ptr:
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

Draft mismatch: switch to $defs (2020-12) and avoid type-name collisions

You advertise 2020-12 but emit legacy definitions/$ref. Also, t.Name() collides across packages. Use $defs and a fully-qualified def key.

-        schema["$ref"] = fmt.Sprintf("#/definitions/%s", t.Name())
+        schema["$ref"] = fmt.Sprintf("#/$defs/%s", defKey(t))
-    schema := map[string]any{
-        "$schema":     "https://json-schema.org/draft/2020-12/schema",
-        "type":        "object",
-        "title":       title,
-        "description": description,
-        "properties":  properties,
-    }
+    schema := map[string]any{
+        "$schema":     "https://json-schema.org/draft/2020-12/schema",
+        "type":        "object",
+        "title":       title,
+        "description": description,
+        "properties":  properties,
+    }
 
-    if len(defs) > 0 {
-        definitions := make(map[string]any)
-        for t, def := range defs {
-            definitions[t.Name()] = def
-        }
-        schema["definitions"] = definitions
-    }
+    if len(defs) > 0 {
+        defsOut := make(map[string]any)
+        for t, def := range defs {
+            defsOut[defKey(t)] = def
+        }
+        schema["$defs"] = defsOut
+    }

Add once in this file:

// defKey builds a stable, unique key for type definitions.
func defKey(t reflect.Type) string {
    if t == nil {
        return ""
    }
    if t.PkgPath() == "" {
        return t.Name()
    }
    key := t.PkgPath() + "." + t.Name()
    key = strings.ReplaceAll(key, "/", ".")
    return key
}

Also applies to: 281-285, 271-279

Comment on lines +459 to +460
ConfigAtom and DataAtom require an AtomMeta parameter (can pass nil if not not needed) to provide schema information for external tools and AI agents. SharedAtom does not use AtomMeta since it's only for internal state sharing.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

Fix grammar and clarify optionality of AtomMeta.

"require" is misleading and there's a double "not". Suggest:

-ConfigAtom and DataAtom require an AtomMeta parameter (can pass nil if not not needed) to provide schema information for external tools and AI agents. SharedAtom does not use AtomMeta since it's only for internal state sharing.
+ConfigAtom and DataAtom accept an optional app.AtomMeta parameter (pass nil when not needed) to provide schema information for external tools and AI agents. SharedAtom does not use AtomMeta since it's only for internal state sharing.
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
ConfigAtom and DataAtom require an AtomMeta parameter (can pass nil if not not needed) to provide schema information for external tools and AI agents. SharedAtom does not use AtomMeta since it's only for internal state sharing.
ConfigAtom and DataAtom accept an optional app.AtomMeta parameter (pass nil when not needed) to provide schema information for external tools and AI agents. SharedAtom does not use AtomMeta since it's only for internal state sharing.
🤖 Prompt for AI Agents
In tsunami/prompts/system.md around lines 459 to 460, the sentence is
grammatically awkward and misleading about AtomMeta optionality: replace
"ConfigAtom and DataAtom require an AtomMeta parameter (can pass nil if not not
needed) to provide schema information for external tools and AI agents.
SharedAtom does not use AtomMeta since it's only for internal state sharing."
with a corrected version that clarifies AtomMeta is optional (e.g., "ConfigAtom
and DataAtom accept an optional AtomMeta parameter (pass nil if not needed) to
provide schema information for external tools and AI agents. SharedAtom does not
use AtomMeta because it is only for internal state sharing.") so that "require"
is changed to "accept an optional" and the duplicated "not" is removed.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 0

♻️ Duplicate comments (4)
tsunami/engine/schema.go (3)

81-86: Pattern tag support: resolved.

Field-level pattern is now applied for string fields. Looks good.


202-208: []byte schema uses contentEncoding/contentMediaType: good.

Correct switch away from nonstandard format: base64.


231-236: Avoid type-name collisions in $ref/$defs; use a fully-qualified def key.

t.Name() can collide across packages and overwrite defs. Use a stable key built from PkgPath()+Name in both $ref and $defs.

Apply this diff in-place:

-        schema["$ref"] = fmt.Sprintf("#/$defs/%s", t.Name())
+        schema["$ref"] = fmt.Sprintf("#/$defs/%s", defKey(t))
-    if len(defs) > 0 {
-        definitions := make(map[string]any)
-        for t, def := range defs {
-            definitions[t.Name()] = def
-        }
-        schema["$defs"] = definitions
-    }
+    if len(defs) > 0 {
+        defsOut := make(map[string]any)
+        for t, def := range defs {
+            defsOut[defKey(t)] = def
+        }
+        schema["$defs"] = defsOut
+    }

Add this helper (outside the ranges above, e.g., below imports):

// defKey builds a stable, unique key for type definitions.
func defKey(t reflect.Type) string {
	if t == nil {
		return ""
	}
	if t.PkgPath() == "" {
		return t.Name()
	}
	key := t.PkgPath() + "." + t.Name()
	return strings.ReplaceAll(key, "/", ".")
}

Also applies to: 279-286

tsunami/prompts/system.md (1)

459-460: Fix grammar and clarify AtomMeta optionality.

Use “accept an optional” and remove the double “not”.

-app.ConfigAtom and app.DataAtom require an app.AtomMeta parameter (can pass nil if not not needed) to provide schema information for external tools and AI agents. app.SharedAtom does not use app.AtomMeta since it's only for internal state sharing.
+app.ConfigAtom and app.DataAtom accept an optional app.AtomMeta parameter (pass nil when not needed) to provide schema information for external tools and AI agents. app.SharedAtom does not use app.AtomMeta because it is only for internal state sharing.
🧹 Nitpick comments (3)
tsunami/engine/schema.go (2)

173-181: Constrain meta.Enum to strings (or coerce types) to avoid invalid schemas.

Right now enum values (strings) are set regardless of field type, which is invalid for numeric/bool schemas. Either gate to strings or parse to the target type.

-    // Add enum values if specified
-    if len(meta.Enum) > 0 {
+    // Add enum values if specified (string-only for now)
+    if schema["type"] == "string" && len(meta.Enum) > 0 {
         enumValues := make([]any, len(meta.Enum))
         for i, v := range meta.Enum {
             enumValues[i] = v
         }
         schema["enum"] = enumValues
     }

If you need numeric enums later, we can add a safe parser.


271-277: Consider locking down top-level to known atoms.

Optionally set additionalProperties: false to help clients validate unknown keys.

   schema := map[string]any{
       "$schema":     "https://json-schema.org/draft/2020-12/schema",
       "type":        "object",
       "title":       title,
       "description": description,
       "properties":  properties,
+      "additionalProperties": false,
   }
tsunami/prompts/system.md (1)

1270-1318: Resolve markdownlint MD010 (hard tabs).

Replace hard tabs with spaces in changed sections to satisfy MD010.

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 4fba48f and 6aa0540.

📒 Files selected for processing (3)
  • tsunami/engine/atomimpl.go (2 hunks)
  • tsunami/engine/schema.go (1 hunks)
  • tsunami/prompts/system.md (7 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
  • tsunami/engine/atomimpl.go
🧰 Additional context used
🧬 Code graph analysis (1)
tsunami/engine/schema.go (3)
tsunami/util/util.go (1)
  • ParseJSONTag (249-285)
tsunami/engine/atomimpl.go (1)
  • AtomMeta (14-21)
tsunami/engine/rootelem.go (1)
  • RootElem (36-49)
🪛 markdownlint-cli2 (0.17.2)
tsunami/prompts/system.md

1235-1235: Hard tabs
Column: 1

(MD010, no-hard-tabs)


1236-1236: Hard tabs
Column: 1

(MD010, no-hard-tabs)


1238-1238: Hard tabs
Column: 1

(MD010, no-hard-tabs)


1270-1270: Hard tabs
Column: 1

(MD010, no-hard-tabs)


1271-1271: Hard tabs
Column: 1

(MD010, no-hard-tabs)


1272-1272: Hard tabs
Column: 1

(MD010, no-hard-tabs)


1274-1274: Hard tabs
Column: 1

(MD010, no-hard-tabs)


1275-1275: Hard tabs
Column: 1

(MD010, no-hard-tabs)


1276-1276: Hard tabs
Column: 1

(MD010, no-hard-tabs)


1277-1277: Hard tabs
Column: 1

(MD010, no-hard-tabs)


1278-1278: Hard tabs
Column: 1

(MD010, no-hard-tabs)


1279-1279: Hard tabs
Column: 1

(MD010, no-hard-tabs)


1280-1280: Hard tabs
Column: 1

(MD010, no-hard-tabs)


1281-1281: Hard tabs
Column: 1

(MD010, no-hard-tabs)


1283-1283: Hard tabs
Column: 1

(MD010, no-hard-tabs)


1284-1284: Hard tabs
Column: 1

(MD010, no-hard-tabs)


1285-1285: Hard tabs
Column: 1

(MD010, no-hard-tabs)


1286-1286: Hard tabs
Column: 1

(MD010, no-hard-tabs)


1287-1287: Hard tabs
Column: 1

(MD010, no-hard-tabs)


1288-1288: Hard tabs
Column: 1

(MD010, no-hard-tabs)


1289-1289: Hard tabs
Column: 1

(MD010, no-hard-tabs)


1290-1290: Hard tabs
Column: 1

(MD010, no-hard-tabs)


1292-1292: Hard tabs
Column: 1

(MD010, no-hard-tabs)


1293-1293: Hard tabs
Column: 1

(MD010, no-hard-tabs)


1294-1294: Hard tabs
Column: 1

(MD010, no-hard-tabs)


1295-1295: Hard tabs
Column: 1

(MD010, no-hard-tabs)


1296-1296: Hard tabs
Column: 1

(MD010, no-hard-tabs)


1297-1297: Hard tabs
Column: 1

(MD010, no-hard-tabs)


1298-1298: Hard tabs
Column: 1

(MD010, no-hard-tabs)


1299-1299: Hard tabs
Column: 1

(MD010, no-hard-tabs)


1300-1300: Hard tabs
Column: 1

(MD010, no-hard-tabs)


1301-1301: Hard tabs
Column: 1

(MD010, no-hard-tabs)


1302-1302: Hard tabs
Column: 1

(MD010, no-hard-tabs)


1303-1303: Hard tabs
Column: 1

(MD010, no-hard-tabs)


1305-1305: Hard tabs
Column: 1

(MD010, no-hard-tabs)


1306-1306: Hard tabs
Column: 1

(MD010, no-hard-tabs)


1307-1307: Hard tabs
Column: 1

(MD010, no-hard-tabs)


1308-1308: Hard tabs
Column: 1

(MD010, no-hard-tabs)


1309-1309: Hard tabs
Column: 1

(MD010, no-hard-tabs)


1310-1310: Hard tabs
Column: 1

(MD010, no-hard-tabs)


1311-1311: Hard tabs
Column: 1

(MD010, no-hard-tabs)


1312-1312: Hard tabs
Column: 1

(MD010, no-hard-tabs)


1313-1313: Hard tabs
Column: 1

(MD010, no-hard-tabs)


1314-1314: Hard tabs
Column: 1

(MD010, no-hard-tabs)


1316-1316: Hard tabs
Column: 1

(MD010, no-hard-tabs)


1317-1317: Hard tabs
Column: 1

(MD010, no-hard-tabs)


1318-1318: Hard tabs
Column: 1

(MD010, no-hard-tabs)


1336-1336: Hard tabs
Column: 1

(MD010, no-hard-tabs)

🔇 Additional comments (2)
tsunami/engine/schema.go (1)

249-258: GetAtomType already returns a concrete reflect.Type; original concern is incorrect.

  • AtomImpl.GetAtomType returns reflect.TypeOf((*T)(nil)).Elem() (tsunami/engine/atomimpl.go:107–109) — this produces a non-nil reflect.Type for any T (including interface types), so atoms won't vanish from the schema due to a nil type.
  • AtomImpl does not store a valType field (MakeAtomImpl initializes lock/val/meta); storing valType is an optional micro-optimization, not required to prevent nil types.

Likely an incorrect or invalid review comment.

tsunami/prompts/system.md (1)

695-695: Link tag fix looks correct.

Using href for stylesheets is the right attribute.

@sawka sawka merged commit 8c47b82 into main Sep 12, 2025
2 of 7 checks passed
@sawka sawka deleted the sawka/tsunami-schema branch September 12, 2025 07:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant